refactor: implement pluggable cache backend architecture#1305
Open
jordpo wants to merge 2 commits intoearth-mover:mainfrom
Open
refactor: implement pluggable cache backend architecture#1305jordpo wants to merge 2 commits intoearth-mover:mainfrom
jordpo wants to merge 2 commits intoearth-mover:mainfrom
Conversation
Refactors the cache layer to support multiple interchangeable backend implementations (in-memory, filesystem, Redis, etc). Key changes: - Add StorageCache trait for pluggable cache backends - Implement EphemeralCache (default, in-memory LRU) and NoOpCache - Add CacheBackend enum to CachingConfig for backend selection - Refactor AssetManager to use trait objects instead of concrete types - Add comprehensive Rust documentation Fully backward compatible - default behavior unchanged. All 104 library tests pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add the missing `backend` field to the `From<&PyCachingConfig>` implementation. This field was added in the cache backend refactor but was missing from the Python bindings conversion, causing a compilation error in the Read the Docs build pipeline. The backend is set to `Default::default()` which uses `CacheBackend::Ephemeral`, the default cache backend. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
Hi @jordpo We recently made some big changes to the main branch, would you mind re-syncing this? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refactors the cache layer to support multiple interchangeable backend implementations, enabling future support for Redis, filesystem, and cloud-native caching strategies.
Motivation
The current cache implementation is tightly coupled to the
quick_cachein-memory LRU cache. This refactoring provides a clean abstraction that allows:Changes
New Cache Module (
icechunk/src/cache/)StorageCachetrait: Object-safe trait defining the cache interfaceEphemeralCache: In-memory LRU cache usingquick_cache(default)NoOpCache: Pass-through cache that performs no cachingCacheBackendenum: Serializable configuration for backend selectionEphemeral(default) - In-memory LRUNoOp- Disables cachingUpdated Components
CachingConfig: Addedbackendfield (defaults toEphemeral)AssetManager: Refactored to useArc<dyn StorageCache>trait objectsDocumentation
Design Decisions
Ephemeralensures zero breaking changes - all existing behavior preservedRepositoryErrorFuture Work
This architecture enables easy addition of new backends:
To add a new backend:
StorageCachetraitCacheBackendenumAssetManager::new()to handle the new variantTesting
✅ All 104 library tests pass, including:
No test modifications were required (except adding the
backendfield to test config structs with default values).Breaking Changes
None - this is a pure internal refactoring with full backward compatibility.
Checklist